Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix beforeEach hook execution order #6673

Merged
merged 1 commit into from
Jul 11, 2018
Merged

fix beforeEach hook execution order #6673

merged 1 commit into from
Jul 11, 2018

Conversation

aaronabramov
Copy link
Contributor

@aaronabramov aaronabramov commented Jul 11, 2018 •
edited
Loading

found a nasty bug where this test:

beforeEach(() => console.log(1));
beforeEach(() => console.log(2));
test('test', () => console.log('test'));

would print:

2
1
test

aaronabramov
aaronabramov commented Jul 11, 2018
View reviewed changes
packages/jest-circus/src/__tests__/__snapshots__/hooks.test.js.snap
run_describe_start: 2nd level describe
test_start: test
hook_start: beforeEach
before each 1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before the fix this would print before each 2 and the next one (line 104) would be before each 1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like these snapshots, really easy to review

rickhanlonii
rickhanlonii approved these changes Jul 11, 2018
View reviewed changes
packages/jest-circus/src/__tests__/__snapshots__/hooks.test.js.snap
run_describe_start: 2nd level describe
test_start: test
hook_start: beforeEach
before each 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like these snapshots, really easy to review

@codecov-io
Copy link

Codecov Report

Merging #6673 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6673      +/-   ##
==========================================
- Coverage   63.63%   63.61%   -0.02%     
==========================================
  Files         235      235              
  Lines        8969     8971       +2     
  Branches        4        3       -1     
==========================================
  Hits         5707     5707              
- Misses       3261     3263       +2     
  Partials        1        1
Impacted Files Coverage Δ
packages/jest-circus/src/utils.js 21.31% <0%> (-0.36%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c32d2d3...eb11199. Read the comment docs.

@aaronabramov aaronabramov merged commit 40b0c6a into jestjs:master Jul 11, 2018
@aaronabramov aaronabramov deleted the beforeEach-hooks branch July 11, 2018 02:39
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rickhanlonii rickhanlonii rickhanlonii approved these changes

Assignees
No one assigned
Labels
cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aaronabramov @codecov-io @rickhanlonii @facebook-github-bot